Skip to content

dataconnect: fix generation of invalid timestamp values in internal test utilities #6689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 13 commits into from
Feb 12, 2025
Merged

dataconnect: fix generation of invalid timestamp values in internal test utilities #6689

merged 13 commits into from
Feb 12, 2025

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Feb 10, 2025 •
edited
Loading

There were some issues when generating Timestamp values for testing. The first was some bad math when generating nanosecond values, where values exceeding the maximum value of 999,999,999 were being generated. The second was a boundary case where the time zone offset would push the second/nanosecond value above the maximum value or below the minimum value. This PR fixes both of those.

@dconeybe dconeybe self-assigned this Feb 10, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 10, 2025 •
edited
Loading

Release note changes

No release note changes were detected. If you made changes that should be
present in the next release, ensure you've added an entry in the appropriate
CHANGELOG.md file(s).

@github-actions GitHub Actions
Copy link
Contributor

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 10, 2025 •
edited
Loading

Coverage Report 1

Affected Products

No changes between base commit (2d1905e) and merge commit (fb28713).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Wu1H42dVjI.html

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 10, 2025 •
edited
Loading

Test Results

   64 files  +   56     64 suites  +56   1m 13s ⏱️ +54s
  542 tests +  520    541 ✅ +  519  1 💤 +1  0 ❌ ±0 
1 084 runs  +1 040  1 082 ✅ +1 038  2 💤 +2  0 ❌ ±0 

Results for commit 96bd96d. ± Comparison against base commit f80ba89.

This pull request removes 22 and adds 542 tests. Note that renamed tests count towards both. 
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists
com.google.firebase.crashlytics.internal.common.DataCollectionArbiterRobolectricTest ‑ testDefaultDataCollection_usedWhenNoOverrideOrManifestSetting
com.google.firebase.crashlytics.internal.common.DataCollectionArbiterRobolectricTest ‑ testManifestMetadata_respectedWhenNoOverride
com.google.firebase.crashlytics.internal.common.DataCollectionArbiterRobolectricTest ‑ testSetCrashlyticsDataCollectionEnabled_overridesOtherSettings
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_notPersistIfAnrBeforeSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_notPersistIfAppExitInfoNotAnrButWithinSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_persistIfAnrWithinSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_persistIfAnrWithinSession_multipleAppExitInfo
…

com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ descriptor should have expected values
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ deserialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ serialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Boolean) creates an object with the expected value
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (normal cases)
…

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 10, 2025 •
edited
Loading

Size Report 1

Affected Products

No changes between base commit (2d1905e) and merge commit (fb28713).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Ix52P1GGIx.html

@dconeybe dconeybe marked this pull request as ready for review February 11, 2025 21:52
@dconeybe dconeybe merged commit caed74c into main Feb 12, 2025
40 of 41 checks passed
@dconeybe dconeybe deleted the dconeybe/dataconnect/JavaTimeArbsNanosecondNumberFormatExceptionFix branch February 12, 2025 02:01
@firebase firebase locked and limited conversation to collaborators Mar 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aashishpatil-g aashishpatil-g aashishpatil-g approved these changes

Assignees

@dconeybe dconeybe

Labels
api: dataconnect main-merge-ack no-changelog size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dconeybe @google-oss-bot @aashishpatil-g @emilypgoogle @rlazo